Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix Basic01 #1282

Merged
merged 3 commits into from
Sep 4, 2023
Merged

Fix Basic01 #1282

merged 3 commits into from
Sep 4, 2023

Conversation

tgreenx
Copy link
Contributor

@tgreenx tgreenx commented Aug 29, 2023 •
edited
Loading

Purpose

This PR updates Basic01 to make use of the correct variables due to an oversight in the initial implementation update (#1212).

This would happen when a referral points to out-of-bailiwick name servers for a zone that isn’t the zone to test, but some parent of it. See comment below.

Context

Fixes #1280

How to test this PR

$ zonemaster-cli --show-testcase --test Basic/basic01 --level INFO --raw 0.0.9.0.0.0.7.0.1.0.0.2.ip6.arpa
   0.00 INFO     UNSPECIFIED    GLOBAL_VERSION   version=v4.7.2
  20.06 INFO     BASIC01        B01_PARENT_FOUND   domain=0.0.7.0.1.0.0.2.ip6.arpa; ns_ip_list=nac.no/128.39.2.22;nac.no/2001:700:0:102::aa53;nn.uninett.no/158.38.0.181;nn.uninett.no/2001:700:0:503::aa:5302;ns.uninett.no/158.38.3.139;ns.uninett.no/2001:700:1:12::3:139;server.nordu.net/193.10.252.19;server.nordu.net/2001:948:4:2::19
  20.07 INFO     BASIC01        B01_CHILD_FOUND   domain=0.0.9.0.0.0.7.0.1.0.0.2.ip6.arpa

matsduf and others added 3 commits July 24, 2023 22:05
@matsduf
Merge pull request zonemaster#1267 from zonemaster/master
4500774 
Merge master into develop (Engine)
@matsduf
Merge pull request zonemaster#1273 from zonemaster/master
c5cdf80 
Merge master into develop (Engine)
@tgreenx
Fix Basic01
6b53599 
Makes use of the correct variables (fixes zonemaster#1280)
@tgreenx tgreenx added A-TestCase Area: Test case specification or implementation of test case V-Patch Versioning: The change gives an update of patch in version. labels Aug 29, 2023
@tgreenx tgreenx added this to the v2023.2 milestone Aug 29, 2023
@marc-vanderwal marc-vanderwal linked an issue Aug 30, 2023 that may be closed by this pull request
BASIC01 does not find parent domain when testing an .arpa-domain #1280
Closed
@matsduf
Copy link
Contributor

matsduf commented Aug 30, 2023

Is this a candidate for a fix version?

@matsduf matsduf added the T-Bug Type: Bug in software or error in test case description label Aug 30, 2023
@matsduf
Copy link
Contributor

matsduf commented Aug 30, 2023 •
edited
Loading

This would impact any zone for which name servers give referral responses with missing glue.

To me "missing" means that there should be glue, but it is not there. If I have understood it correctly, it impacts a zone for which the referrals are without glue, which is how it should be in many cases.

Especially for reverse zones, in-bailiwick NS are not expected. If there is no in-bailiwick NS, glue is not needed.

@matsduf matsduf changed the base branch from develop to v2023.1.4 August 30, 2023 11:56
@matsduf matsduf modified the milestones: v2023.2, v2023.1.4 Aug 30, 2023
@matsduf
Copy link
Contributor

matsduf commented Aug 31, 2023

Can this PR be merged?

@tgreenx tgreenx merged commit 8964a50 into zonemaster:releases/v2023.1.4 Sep 4, 2023
3 checks passed
@tgreenx tgreenx deleted the fix-basic01 branch September 4, 2023 08:14
@matsduf
Copy link
Contributor

matsduf commented Sep 4, 2023

@tgreenx, can the description be updated to make it clearer what zones were impacted. I feel "any zone for which name servers give referral responses with missing glue" is not fully correct. See 0-24.se which has no glue in delegation (referral) but it is not impacted.

@matsduf matsduf mentioned this pull request Sep 4, 2023
Merged
@marc-vanderwal
Copy link
Contributor

@tgreenx, can the description be updated to make it clearer what zones were impacted. I feel "any zone for which name servers give referral responses with missing glue" is not fully correct. See 0-24.se which has no glue in delegation (referral) but it is not impacted.

I’ve included a suggestion here: zonemaster/zonemaster#1201 (review)

@matsduf matsduf mentioned this pull request Sep 4, 2023
Merged
@marc-vanderwal
Copy link
Contributor

Just to make things clear: the issue only happens when a referral points to out-of-bailiwick name servers for a zone that isn’t the zone to test, but some parent of it.

Firstly, testing ip6.arpa does not exhibit the problem because the first delegation is from the root to arpa, which has in-bailiwick name servers with glue, then to ip6.arpa—the zone we are testing.

Secondly, testing 7.0.1.0.0.2.ip6.arpa also does not exhibit the problem, because the delegation from arpa to ip6.arpa has out-of-bailiwick name servers but contains glue records. Then, there is a delegation from ip6.arpa to 7.0.1.0.0.2.ip6.arpa—the zone we are testing.

Finally, testing 0.0.9.0.0.0.7.0.1.0.0.2.ip6.arpa does exhibit the problem. That’s because the delegation from ip6.arpa to 7.0.1.0.0.2.ip6.arpa, which is a parent of the zone to test, has out-of-bailiwick name servers without glue records. The code then failed to obtain IPv4 and IPv6 addresses for the listed name servers.

In short, the presence or lack of glue was not a problem if the referral pointed to the zone under test. If not, then the bug happened.

@tgreenx tgreenx mentioned this pull request Sep 7, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@marc-vanderwal marc-vanderwal marc-vanderwal approved these changes

@matsduf matsduf matsduf approved these changes

@mattias-p mattias-p Awaiting requested review from mattias-p

@hannaeko hannaeko Awaiting requested review from hannaeko

Assignees
No one assigned
Labels
A-TestCase Area: Test case specification or implementation of test case T-Bug Type: Bug in software or error in test case description V-Patch Versioning: The change gives an update of patch in version.
Projects
None yet
Milestone
v2023.1.4
Development

Successfully merging this pull request may close these issues.

BASIC01 does not find parent domain when testing an .arpa-domain
3 participants
@tgreenx @matsduf @marc-vanderwal